upcoming: [M3-9829] - Refactor AddLinodeDrawer to remove useEffect #12220
Conversation
| if (linodesNeedingInterfaceSelection.length > 0) { | ||
| interfaceError = | ||
| 'You must select an interface to assign to this Firewall.'; | ||
| } |
There was a problem hiding this comment.
the one visual/functionality difference for adding interfaces that (should - unless there are bugs 🤞) exists between this version and the old is that now, if we select a LInode but don't select a corresponding interface, we receive an error (instead of submit button being disabled):
| Before | After |
|---|---|
 |
 |
this aligns behavior with other drawers/forms in CM a bit more (submitting + receiving error back), but I can look into changing back to disabling the button if wanted!
| const linodesNeedingInterfaceSelection = | ||
| selectedLinodesWithMultipleInterfaces.filter( | ||
| (linode) => !selectedIfacesToAdd[linode.id] | ||
| ); | ||
| const failedLinodes: Linode[] = [...linodesNeedingInterfaceSelection]; |
There was a problem hiding this comment.
If we've selected a Linode with multiple eligible interfaces, haven't selected an interface for that Linode, and click add, no interface device gets added for those Linodes. So, we will save them for the next attempt and display a message to select the interfaces
coliu-akamai
requested review from
hana-akamai,
hasyed-akamai and
bnussman-akamai
and removed request for
a team
...features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/LinodeInterfaceFirewall.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Firewalls/FirewallDetail/Devices/utils.ts
Outdated
Show resolved
Hide resolved
bnussman-akamai
added
Add'tl Approval Needed
hasyed-akamai
added
Approved
|
putting this back in draft - after PM feedback, will need to update this to not include multi-firewall assigning ability |
coliu-akamai
changed the title
Cloud Manager UI test results🔺 2 failing tests on test run #14 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/objectStorage/access-key.e2e.spec.ts,cypress/e2e/core/stackscripts/create-stackscripts.spec.ts" |
||||||||||||||||||||
Description 📝
Adds ability to assign multiple firewalls to an entity from the Firewall details page - this functionality always existed in the API, now bringing a bit of parity to CMUpdate Linode Interface table to have a 'Show More' if there exist additional firewallsFOLLOW UP: M3-9984 to continue supporting multiple firewall functionality
RELATED: M3-9884
Changes 🔄
Add ability to assign multiple firewalls for all device types (linode, interface, nodebalancer)Preview 📷
there should be no visual changes (besides comment below)
Functionality changes - see video
cannot-multi-assign.mov
can-multi-assign.mov
How to test 🧪
To test Linode Interface stuff, you'll need the tag + feature flag
Verification steps
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅